src/patches/themes.ts (2)

122-132: objPrefix via indexOf('{') is safe for current themes but brittle.

objMatch[0].slice(0, objMatch[0].indexOf('{')) relies on the first { being the object-literal opener. Because objPat only accepts values starting with Auto |Dark|Light|Monochrome followed by [^"]*, a theme name containing { would split the prefix incorrectly. No current CC theme does this, so it's a latent concern rather than a bug — but using the capture group of the prefix alternation would be more robust:

♻️ Use an explicit capture for the prefix

   const objPat =
-    /(?:return|[$\w]+=)\{(?:"?(?:[$\w-]+)"?:"(?:Auto |Dark|Light|Monochrome)[^"]*",?)+\}/;
+    /(return|[$\w]+=)\{(?:"?(?:[$\w-]+)"?:"(?:Auto |Dark|Light|Monochrome)[^"]*",?)+\}/;
   const objMatch = oldFile.match(objPat);

   if (!objMatch || objMatch.index == undefined) {
     console.error('patch: themes: failed to find objMatch');
     return null;
   }

-  // Preserve the original prefix (either "return" or "VARNAME=")
-  const objPrefix = objMatch[0].slice(0, objMatch[0].indexOf('{'));
+  // Preserve the original prefix (either "return" or "VARNAME=")
+  const objPrefix = objMatch[1];

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/patches/themes.ts` around lines 122 - 132, The current extraction of
objPrefix uses objMatch[0].slice(0, objMatch[0].indexOf('{')), which can break
if a theme value contains a '{'; update the regex objPat to capture the prefix
(the alternation that yields "return" or the variable assignment) as a dedicated
capture group and then set objPrefix from that capture (e.g., use objMatch[1]
instead of slicing), ensuring you still validate objMatch exists before reading
the capture; keep references to objPat, objMatch and objPrefix when making the
change.

---

92-92: objArrItemCount regex counts both value: and "value": occurrences — confirm no collisions inside label text.

(objArrMatch[0].match(/"?value"?:/g) || []).length counts item entries assuming value: appears once per item. That holds for the known CC emissions ("Dark mode", "Light mode", "Auto (match terminal)", "Monochrome"), but any future label containing the literal substring value: would inflate the count and skip the assembly-prefix branch. A stricter count like \{"?label"?:/g (matching the object-opening prefix) would avoid that class of false positive with no extra cost.

-  const objArrItemCount = (objArrMatch[0].match(/"?value"?:/g) || []).length;
+  const objArrItemCount = (objArrMatch[0].match(/\{"?label"?:/g) || []).length;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/patches/themes.ts` at line 92, The current count in objArrItemCount uses
a permissive pattern that matches both value: and "value": and can be inflated
by label text containing that substring; update the counting logic that computes
objArrItemCount from objArrMatch[0] to detect actual object entries by matching
the object-opening label key instead (i.e., count occurrences of the object
prefix for label rather than value), for example by using a pattern that looks
for '{"label":' or unquoted label keys, and keep the rest of the branch logic
(the assembly-prefix decision) unchanged so it reliably reflects the number of
items.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

src/patches/themes.ts (2)

107-110: Minor: {2,} may be slightly over-constrained.

The assembly pattern requires ≥2 intermediate theme identifiers between ...autoVar and the next spread. Current CC has 6+ built-ins (DH,YH,PH,t,o,_H), so this works and is unlikely to overmatch. If a future CC release ever reduces built-ins to 1 (e.g. [...HH,DH,...U.map(...)]), detection will fail-safe to null with the clear error you added — not a regression, just something to keep in mind. Relaxing to {1,} would be more forgiving at the (small) cost of stricter reliance on the trailing ,\.\.\. anchor; either is defensible.

The ReDoS warning from the static analyzer on this new RegExp is a false positive: autoVarName is bounded by [A-Za-z_$][\w$]* and the only metachar that could appear ($) is explicitly escaped, so the constructed pattern is always safe.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/patches/themes.ts` around lines 107 - 110, The assemblyPat RegExp in
src/patches/themes.ts is too strict by requiring at least two intermediate
identifiers between the ...autoVar spread and the next spread; update the
pattern used to build assemblyPat (the RegExp constructed around autoVarName) to
allow one or more intermediates (change the quantifier from {2,} to {1,}) so
assemblies with a single built-in identifier are matched; leave the escaped
metacharacters and surrounding anchors intact (no other logic changes) to
preserve safety against ReDoS.

---

196-217: Add test coverage for the assembly prefix path.

The splicing logic traces correctly: the replaced span ends at the comma before ...U.map(...), and the emitted fragment [{...themes...}, joins cleanly with the untouched suffix, yielding the expected output. The legacy path (full-array replacement) is preserved when isAssemblyPrefix is falsy.

However, themes.ts has no corresponding test file, unlike other patch files (systemPrompts.test.ts, voiceMode.test.ts, etc.). Since both the regex detection and the assembly splicing are critical to correct operation, a test fixture covering both the assembly path (with isAssemblyPrefix: true) and the legacy path would harden this against future Claude Code format changes.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/patches/themes.ts` around lines 196 - 217, Add unit tests for
src/patches/themes.ts to cover both assembly-prefix and legacy replacement
paths: create fixtures exercising the code that computes objArr (use themes.map
-> themeItems), toggling locations.objArr.isAssemblyPrefix true and false, and
assert the produced newFile fragments join correctly with the untouched suffix
(verify the emitted open-array case ends with a comma and the full-array case
contains the closing bracket). Target the functions/logic around
locations.objArr, objArr, and the showDiff call so the regex detection and
splicing behavior are validated, mirroring existing test patterns used in
systemPrompts.test.ts and voiceMode.test.ts.